feat: Improve impersonation logic #26891
Conversation
|
Size Change: 0 B Total Size: 1.11 MB ℹ️ View Unchanged
|
| @@ -677,7 +677,19 @@ def get_impersonated_session_expires_at(request: HttpRequest) -> Optional[dateti | |||
|
|
|||
| init_time = get_or_set_session_cookie_created_at(request=request) | |||
|
|
|||
| return datetime.fromtimestamp(init_time) + timedelta(seconds=settings.IMPERSONATION_TIMEOUT_SECONDS) | |||
| last_activity_time = request.session.get(settings.IMPERSONATION_COOKIE_LAST_ACTIVITY_KEY, init_time) | |||
There was a problem hiding this comment.
thinking out loud
a mini google suggests the default session engine signs the cookie content so a naughty person can't send their own init time here
(and anyway it would only be useful to an attacker if they had control of an impersonated session which would mean even if you could edit this then it wouldn't matter since they have impersonation)
There was a problem hiding this comment.
I think you're confusing session and cookies. The cookie just has a sessionid. The session is loaded in django from the DB. So here it is modifying the session DB entry, nothing from the user.
There was a problem hiding this comment.
Awesome, bad googling from me 👍
|
survey test fixes in #26892 |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Problem
Enough discussion around the limits of our current impersonation logic - lets improve it
Changes
Manual refreshing
Automatic logout check
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?